Skip to content

Add MCP tool annotations#81

Merged
RadCod3 merged 2 commits into
mainfrom
feat/add-tool-annotations
Apr 25, 2026
Merged

Add MCP tool annotations#81
RadCod3 merged 2 commits into
mainfrom
feat/add-tool-annotations

Conversation

@RadCod3

@RadCod3 RadCod3 commented Apr 25, 2026

Copy link
Copy Markdown
Owner

$subject
image

Resolves #80

@coderabbitai

coderabbitai Bot commented Apr 25, 2026

Copy link
Copy Markdown
📝 Walkthrough

Summary by CodeRabbit

  • Refactor

    • Centralized tool annotation management with new helper functions for read-only and mutating operations.
    • Updated tool decorators across accounts, budgets, insights, and transactions modules with consistent metadata.
  • Tests

    • Added test suite validating annotation metadata for all tools.

Walkthrough

A new helper module is introduced to centralize MCP tool annotation creation with two functions: readonly_annotations() for read-only tools and mutating_annotations() for mutating operations. These helpers are then applied consistently across accounts, budgets, insights, and transactions tool modules to standardize metadata, accompanied by comprehensive test coverage.

Changes

Cohort / File(s) Summary
Annotation Helpers
src/lampyrid/tools/_annotations.py
New module defining readonly_annotations(title) and mutating_annotations(title, destructive, idempotent) helper functions to construct standardized ToolAnnotations with hint configuration.
Tool Modules with Annotations
src/lampyrid/tools/accounts.py, src/lampyrid/tools/budgets.py, src/lampyrid/tools/insights.py, src/lampyrid/tools/transactions.py
Apply annotation helpers to tool decorators: read-only endpoints use readonly_annotations(), mutating endpoints use mutating_annotations() with appropriate destructive and idempotent flags set per operation.
Annotation Tests
tests/unit/test_tool_annotations.py
New test module validating MCP tool annotations across all four tool servers, asserting title, readOnlyHint, destructiveHint, idempotentHint, and openWorldHint fields per tool.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Annotations bloom like carrots in spring,
With read-only hints and destructive zing,
Four tool gardens now wear their new tags,
Tidy and tested, no more metadata drags!
The MCP world shines a little more bright. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is vague and generic, using only the placeholder '$subject' which does not convey any meaningful information about the changeset. Replace the placeholder with a meaningful description explaining what MCP annotations were added, why they were needed, and their impact.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add MCP tool annotations' directly and clearly summarizes the main change—introducing annotation support for MCP tools across multiple modules.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/add-tool-annotations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.66%. Comparing base (011d2f4) to head (209d8f6).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #81   +/-   ##
=======================================
  Coverage   97.66%   97.66%           
=======================================
  Files          19       20    +1     
  Lines        3121     3130    +9     
=======================================
+ Hits         3048     3057    +9     
  Misses         73       73           
Flag Coverage Δ
integration 97.66% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
tests/unit/test_tool_annotations.py (1)

32-46: Consider parametrizing idempotent in the helper.

_assert_mutating_tool hard-codes idempotentHint is False, which couples the helper to the current set of callsites. If any mutating tool is later marked idempotent=True (e.g., updates — see comment on transactions.py), this helper will need a kwarg. A small forward-looking change keeps things symmetric with destructive:

♻️ Proposed change
 def _assert_mutating_tool(
     tool: Tool,
     title: str,
     *,
     destructive: bool = False,
+    idempotent: bool = False,
 ) -> None:
     """Assert standard annotations for mutating tools."""
     annotations = tool.annotations

     assert annotations is not None
     assert annotations.title == title
     assert annotations.readOnlyHint is False
     assert annotations.destructiveHint is destructive
-    assert annotations.idempotentHint is False
+    assert annotations.idempotentHint is idempotent
     assert annotations.openWorldHint is False
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/test_tool_annotations.py` around lines 32 - 46, The helper
_assert_mutating_tool currently hard-codes annotations.idempotentHint as False;
add an idempotent: bool = False parameter to _assert_mutating_tool, update its
docstring to mention idempotency, and replace the hard-coded assertion with
assert annotations.idempotentHint is idempotent so callers can opt-in when a
mutating tool is idempotent.
src/lampyrid/tools/transactions.py (1)

142-163: Consider marking updates as idempotent.

Per the MCP idempotentHint semantics ("calling repeatedly with the same arguments has no additional effect on the environment"), update_transaction and bulk_update_transactions are idempotent — applying the same payload twice produces the same end state. The mutating_annotations helper already exposes this flag but no callsite sets it. This will require updating the test expectations (_assert_mutating_tool currently hard-asserts idempotentHint is False for all mutating tools).

♻️ Proposed change
     `@transactions_mcp.tool`(
         tags={'transactions', 'manage'},
-        annotations=mutating_annotations('Update Transaction'),
+        annotations=mutating_annotations('Update Transaction', idempotent=True),
     )
     async def update_transaction(req: UpdateTransactionRequest) -> Transaction:
@@
     `@transactions_mcp.tool`(
         tags={'transactions', 'manage', 'bulk'},
-        annotations=mutating_annotations('Bulk Update Transactions'),
+        annotations=mutating_annotations('Bulk Update Transactions', idempotent=True),
     )
     async def bulk_update_transactions(req: BulkUpdateTransactionsRequest) -> BulkUpdateResult:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lampyrid/tools/transactions.py` around lines 142 - 163, The mutating
tools update_transaction and bulk_update_transactions should be marked
idempotent via the existing mutating_annotations helper: change the calls to
mutating_annotations('Update Transaction') and mutating_annotations('Bulk Update
Transactions') to pass the idempotent flag (e.g., mutating_annotations('Update
Transaction', idempotent=True) and mutating_annotations('Bulk Update
Transactions', idempotent=True)); also update the test helper
_assert_mutating_tool to stop hard-asserting idempotentHint is False (make it
accept a configurable expectation or assert True for these tools) so the tests
reflect the idempotent semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lampyrid/tools/transactions.py`:
- Around line 142-163: The mutating tools update_transaction and
bulk_update_transactions should be marked idempotent via the existing
mutating_annotations helper: change the calls to mutating_annotations('Update
Transaction') and mutating_annotations('Bulk Update Transactions') to pass the
idempotent flag (e.g., mutating_annotations('Update Transaction',
idempotent=True) and mutating_annotations('Bulk Update Transactions',
idempotent=True)); also update the test helper _assert_mutating_tool to stop
hard-asserting idempotentHint is False (make it accept a configurable
expectation or assert True for these tools) so the tests reflect the idempotent
semantics.

In `@tests/unit/test_tool_annotations.py`:
- Around line 32-46: The helper _assert_mutating_tool currently hard-codes
annotations.idempotentHint as False; add an idempotent: bool = False parameter
to _assert_mutating_tool, update its docstring to mention idempotency, and
replace the hard-coded assertion with assert annotations.idempotentHint is
idempotent so callers can opt-in when a mutating tool is idempotent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 91292d0e-00fb-4fd7-bd03-060ec4bdf98d

📥 Commits

Reviewing files that changed from the base of the PR and between 011d2f4 and 209d8f6.

📒 Files selected for processing (6)
  • src/lampyrid/tools/_annotations.py
  • src/lampyrid/tools/accounts.py
  • src/lampyrid/tools/budgets.py
  • src/lampyrid/tools/insights.py
  • src/lampyrid/tools/transactions.py
  • tests/unit/test_tool_annotations.py

@RadCod3 RadCod3 merged commit fa750c1 into main Apr 25, 2026
5 checks passed
@RadCod3 RadCod3 deleted the feat/add-tool-annotations branch April 25, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add mcp tool annotations

2 participants